Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CL-EL withdrawals harmonization: using units of Gwei #354

Merged
merged 2 commits into from
Jan 17, 2023
Merged

CL-EL withdrawals harmonization: using units of Gwei #354

merged 2 commits into from
Jan 17, 2023

Conversation

ralexstokes
Copy link
Member

Following discussion from here ethereum/pm#702, protocol devs want to explore harmonization in the withdrawals format from CL to EL.

This PR updates the Engine API with the first part of this change to use the same units (Gwei) for the amount in each withdrawal.

@ralexstokes
Copy link
Member Author

See EIP update here: ethereum/EIPs#6325

@timbeiko timbeiko mentioned this pull request Jan 12, 2023
Closed
@terencechain terencechain mentioned this pull request Jan 12, 2023
Merged
@MarekM25 MarekM25 mentioned this pull request Jan 13, 2023
Merged
@g11tech g11tech mentioned this pull request Jan 13, 2023
Merged
@yperbasis yperbasis mentioned this pull request Jan 13, 2023
Merged
djrtwo
djrtwo approved these changes Jan 13, 2023
View reviewed changes
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! not sure why line 146 is in the diff though

src/engine/shanghai.md Show resolved Hide resolved
g11tech
g11tech approved these changes Jan 13, 2023
View reviewed changes
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@g11tech g11tech mentioned this pull request Jan 13, 2023
Merged
@tersec
Copy link
Contributor

tersec commented Jan 13, 2023

lgtm

tersec added a commit to status-im/nim-web3 that referenced this pull request Jan 14, 2023
@tersec
withdrawals amount in uint64 gwei: ethereum/execution-apis#354
b5188b0
tersec added a commit to status-im/nim-eth that referenced this pull request Jan 14, 2023
@tersec
withdrawals amount in uint64 gwei: ethereum/execution-apis#354
56da74d
@tersec tersec mentioned this pull request Jan 14, 2023
Merged
tersec added a commit to status-im/nim-eth that referenced this pull request Jan 14, 2023
@tersec
withdrawals amount in uint64 gwei: ethereum/execution-apis#354 (#577)
68f4c53
@tersec tersec mentioned this pull request Jan 14, 2023
Merged
This was referenced Jan 14, 2023
Closed
Closed
etan-status
etan-status approved these changes Jan 14, 2023
View reviewed changes
Copy link
Contributor

@etan-status etan-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. This will allow eventually migrating the EL's withdrawals_root and txs_root to SSZ, eliminating the inconsistency with CL ExecutionPayloadHeader values.

@ethDreamer ethDreamer mentioned this pull request Jan 15, 2023
Merged
etan-status added a commit to etan-status/consensus-specs that referenced this pull request Jan 15, 2023
@etan-status
Use Gwei for Withdrawal amount in mock-EE
52d978b 
Updates the EL block hash computation for pytests to match latest spec.
Notably, use `Gwei` for `Withdrawal` amount consistently.
Also fix `excess_data_gas` not being correctly accounted for.

ethereum/execution-apis#354
ethereum/EIPs#6325
@etan-status etan-status mentioned this pull request Jan 15, 2023
Merged
potuz
potuz approved these changes Jan 16, 2023
View reviewed changes
Copy link
Contributor

@potuz potuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get this merged today so that we can merge in production and use production code in the devnet?

@usmansaleem usmansaleem mentioned this pull request Jan 16, 2023
Merged
2 tasks
@lightclient lightclient merged commit 5561353 into ethereum:main Jan 17, 2023
@ralexstokes ralexstokes deleted the withdrawals-gwei-arc branch January 17, 2023 05:00
yperbasis added a commit to erigontech/erigon that referenced this pull request Jan 17, 2023
@yperbasis
Withdrawal amount in GWei (#6578)
f151a52 
See ethereum/execution-apis#354 &
ethereum/EIPs#6325.

Prerequisite: ledgerwatch/erigon-lib#832.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terencechain terencechain terencechain left review comments

@timbeiko timbeiko timbeiko left review comments

@etan-status etan-status etan-status approved these changes

@g11tech g11tech g11tech approved these changes

@yperbasis yperbasis yperbasis approved these changes

@potuz potuz potuz approved these changes

@lightclient lightclient lightclient approved these changes

@tersec tersec tersec approved these changes

@mkalinin mkalinin mkalinin approved these changes

@djrtwo djrtwo djrtwo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.